Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/apply.go`:
- Around line 55-60: The mode selection in the apply command is bypassing the
standard flag resolution and allowing conflicting inputs to silently override
each other. Update the apply flow around mode/watch handling to use the same
Explicit flag > OPENSHELL_* env var > config file > default pattern used by
gatewayName, and validate the resolved mode in the apply command itself before
assigning it to agentCfg.Mode. Also, when both the mode flag and watch flag are
provided, detect the conflict explicitly and return an error instead of letting
watch override the user’s chosen mode.

In `@cmd/orchestrator/main.go`:
- Around line 38-50: Treat SIGINT/SIGTERM as a clean shutdown at the CLI
boundary by updating cmd/orchestrator/main.go so the error returned from
orch.Run(ctx) is mapped to nil when it is context.Canceled; otherwise keep the
existing non-zero exit path. Use the ctx from signal.NotifyContext and the
orch.Run entrypoint to locate the change, and ensure only real failures still
print an error and exit 1 while user-initiated cancellation exits successfully.

In `@internal/orchestrator/adapter.go`:
- Around line 40-52: CodexAdapter.BuildCommand still uses unsupported flags for
task execution, so switch the task-run path to the documented codex exec
subcommand instead of passing --print or -p. Update BuildCommand in CodexAdapter
so non-empty tasks are invoked via codex exec with the prompt argument, while
preserving the empty-task behavior if needed.

In `@internal/orchestrator/config.go`:
- Around line 40-59: The defaulting logic in OrchestratorConfig.ApplyDefaults is
inconsistent because Heartbeat only resets on 0 while PollInterval and
MaxFailures also handle negative values. Update ApplyDefaults to treat Heartbeat
the same way as the other numeric fields so malformed negative values are
replaced with the default, and keep the behavior aligned with Validate and
downstream timer use such as sleepWithHeartbeat.

In `@internal/orchestrator/orchestrator.go`:
- Around line 206-222: The runHooks method blocks the watch loop because it
executes each hook synchronously with cmd.Run() and no timeout or cancellation
path. Update Orchestrator.runHooks to accept and use a context or derive a
timeout when creating the exec.Command, so hung hooks can be interrupted instead
of blocking indefinitely. Keep the existing hook selection logic for
result.Status and preserve logging in logf, but ensure each hook execution is
bounded and can honor shutdown/cancellation signals.
- Around line 38-106: Update Orchestrator.Run and runWatch so terminal failures
are surfaced as errors instead of success. In runWatch, keep treating
IsTerminal() as a stop condition, but return an error when result.Status is
"terminal_failure" (using result.Reason for context) and only return nil for
successful terminal states. In the "once" branch of Run, also check
result.Status alongside ExitCode so a sentinel terminal_failure with exit code 0
is rejected; use the existing runCycle, IsTerminal, and result.Status/Reason
fields to locate the logic.
- Around line 108-167: runCycle ignores ctx while launching the agent
subprocess, so cancellation and SIGTERM/Ctrl+C won’t interrupt an active cycle.
Update Orchestrator.runCycle to thread ctx into the command creation path,
ideally through adapter.BuildCommand, so cmd.Run() is context-aware. If
BuildCommand cannot accept ctx, add explicit handling for ctx.Done() that
terminates the subprocess (or its process group) during the active run. Keep the
existing sentinel parsing and SessionRecord flow unchanged.

In `@Makefile`:
- Around line 37-41: The orchestrator build target is hardcoding a linux/amd64
binary, which breaks native-arch sandbox builds on arm64 hosts. Update the
Makefile’s orchestrator target so it builds for the host architecture instead of
forcing GOARCH=amd64, and keep the output path/name used by dev-sandbox and
executor.go unchanged so the existing command lookup still works. Use the
orchestrator target as the fix point and align it with the native-arch behavior
implied by the Dockerfile and dev-sandbox flow.

---

Outside diff comments:
In `@cmd/executor.go`:
- Around line 119-128: The Sandbox command selection in cmd/executor.go
currently prefers harness-orchestrator whenever it exists, but that path cannot
start with entrypoint=bash because OrchestratorConfig.Validate only allows
claude, codex, and opencode. Update the sandboxCmd logic to check entrypoint
compatibility before choosing harness-orchestrator, so bash (and any other
unsupported entrypoint) falls back to run.sh instead of failing at LoadConfig.
If you want orchestrator to handle bash directly, then also extend the
allow-list in internal/orchestrator/config.go and keep BuildOrchestratorConfig
in internal/agent/agent.go aligned with that support.

---

Duplicate comments:
In `@internal/orchestrator/adapter_test.go`:
- Around line 49-63: Update the Codex adapter test expectations so they match
the corrected CLI built by CodexAdapter.BuildCommand; the current assertions in
TestCodexAdapterCommands still hard-code the old print flags. Adjust the
expected command strings to reflect the new codex exec invocation for both
headless and tty cases, using the BuildCommand and CodexAdapter symbols to
verify the updated args.

---

Nitpick comments:
In `@internal/orchestrator/config.go`:
- Around line 61-73: Validate the numeric fields in OrchestratorConfig.Validate
in addition to Mode and Entrypoint: add bounds checks for PollInterval,
MaxFailures, and Heartbeat so negative or otherwise invalid values are rejected
after defaults are applied. Update Validate to return an error when these fields
are non-positive or out of range, keeping the existing checks for Mode and
Entrypoint intact.

In `@internal/orchestrator/orchestrator_test.go`:
- Around line 99-170: The watch tests in TestRunWatchStopsOnComplete and
TestRunWatchContextCancel only cover cancellation during the sleep interval and
do not verify terminal-failure handling. Add a test around Orchestrator.Run that
uses the mockAdapter path with a long-running command so context cancellation
interrupts an in-flight cmd.Run(), and add another assertion using
multiMockAdapter’s terminal-failure sentinel to confirm Run() returns an error
for "terminal_failure". Keep the coverage focused on the watch flow and the
status handling in Run, without relying on the instant-echo behavior.

In `@Makefile`:
- Around line 99-104: The dev-sandbox target currently masks cp failures with
2>/dev/null || true, which can hide real write or permission issues before
docker build. Update the dev-sandbox recipe to let cp fail visibly and surface
the problem at the copy step, using the dev-sandbox and orchestrator target flow
to locate it. Keep the binary copy into profiles/images/sandbox-default/
explicit so any missing bin/harness-orchestrator or destination write failure is
reported immediately rather than later by the Dockerfile COPY.